Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: object and op deletion backend + basic sdk + basic fe #2319

Draft
wants to merge 78 commits into
base: master
Choose a base branch
from

Conversation

gtarpenning
Copy link
Member

@gtarpenning gtarpenning commented Sep 5, 2024

Combo pr for testing / draft review

https://wandb.atlassian.net/browse/WB-20784

This pr:

  • adds a flexible obj_delete api that can delete specific versions of an object, or all versions of the object.
  • server api inserts an object with identical primary keys and a deleted_at field. Before insert, we read from the db, and insert the full data payload, ensuring no data is lost. ReplacingMergeTree deduplicates at merge, keeping the latest version of each row grouped by the primary (order by) key.
    • "The most recently created part (the last insert) will be the last one in the selection. Thus, after deduplication, the very last row from the most recent insert will remain for each unique sorting key." link
  • updates the read path to filter out objects with deleted_at set.
  • the client in this pr catches the deleted object error when de-reffing nested objects, instead of throwing.
  • adds a trash button to the object versions page
  • updates the objects query to refetch on delete

TODO:

  • add restriction on deleting object versions to no more than 100 at a time -- we do read the val_dump here so its important to add a sane limit.
  • investigate performance implications for massive data deletion requests. Example: try to delete an object with 100,000 versions. This should be fine without batching, as we do an objects query without grabbing val_dump, and deletion occurs as an insertion.

Api usage:

api = weave.init()
obj = obj.get()

# delete a single version of an object
api.delete_object_version(obj)
# delete all versions of an object
api.delete_object_all_versions(obj.object_id)

Depends on:

Screenshot 2024-09-09 at 3 53 03 PM

delete-obj-1

Deleted object in the call detail view:
Screenshot 2024-12-11 at 11 52 36 AM

Deleting in the calls table (refresh needs some work):
test-delete

@circle-job-mirror
Copy link

circle-job-mirror bot commented Sep 5, 2024

@gtarpenning gtarpenning changed the title Griffin/objs delete feat: object deletion Sep 5, 2024
@gtarpenning gtarpenning changed the title feat: object deletion feat: object and op deletion backend + basic sdk Sep 10, 2024
Comment on lines 537 to 564
const DialogContent = styled(MaterialDialogContent)`
padding: 0 32px !important;
`;
DialogContent.displayName = 'S.DialogContent';

const DialogTitle = styled(MaterialDialogTitle)`
padding: 32px 32px 16px 32px !important;

h2 {
font-weight: 600;
font-size: 24px;
line-height: 30px;
}
`;
DialogTitle.displayName = 'S.DialogTitle';

const DialogActions = styled(MaterialDialogActions)<{$align: string}>`
justify-content: ${({$align}) =>
$align === 'left' ? 'flex-start' : 'flex-end'} !important;
padding: 32px 32px 32px 32px !important;
`;
DialogActions.displayName = 'S.DialogActions';

const DeleteModal: React.FC<{
object: ObjectVersionSchema;
open: boolean;
onClose: () => void;
}> = ({object, open, onClose}) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: dry this with the call delete modal

if (!v) {
// Value for ref not found, probably deleted
refValues[r] = {
_weave_is_deleted_ref: objectRefDisplayName(parseRef(r)).label,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is terrible, we need a better way of storing/rendering deleted refs in the object viewer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants